Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix(patch): fix #746, check desc get is null and only patch window.resize additionally #747

Merged
merged 7 commits into from
Apr 25, 2017
Merged

fix(patch): fix #746, check desc get is null and only patch window.resize additionally #747

merged 7 commits into from
Apr 25, 2017

Conversation

JiaLiPassion
Copy link
Collaborator

fix #746,fix #741

  1. fix: resolve errors with closure #722 break window.onresize is not intercepted by zones #741, not patch onresize(maybe others), so I add it back in this PR
  2. fix Cannot read property 'apply' of undefined #746, check originalDescGet is null or not.

mhevery
mhevery reviewed Apr 22, 2017
View reviewed changes
lib/common/utils.ts Outdated
if (prop.substr(0, 2) == 'on') {
onProperties.push(prop);
}
const prop = 'on' + properties[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have an explicit list rather than iterating over everything? The issue is that closure compiler creates lots of properties on the window with mangled names, and it is quite easy for them to start with on

Copy link
Collaborator Author

@JiaLiPassion JiaLiPassion Apr 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, I will first add on resize and figure out what others are needed.

guodong
guodong approved these changes Apr 22, 2017
View reviewed changes
Copy link

@guodong guodong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve

@JiaLiPassion
Copy link
Collaborator Author

@mhevery , I have updated my PR.

  1. only patch specified properties
  2. window patch resize property
  3. check originalDesc is null or not

please review. thank you !

@JiaLiPassion JiaLiPassion changed the title fix(patch): fix #746, need to patch all on properties without duplicate fix(patch): fix #746, check desc get is null and only patch window.resize additionally Apr 22, 2017
This was referenced Apr 22, 2017
Merged
Closed
@mpalourdio mpalourdio mentioned this pull request Apr 22, 2017
Closed
2 tasks
@smorhaim
Copy link

I tried the patch, it still gives the same error. Same steps to reproduce. Brand new project, ng serve, error.

@JiaLiPassion
Copy link
Collaborator Author

@smorhaim , you can try the attached zone.js, the error still exists because the dist/ folder is not updated.
dist.zip

@smorhaim
Copy link

@JiaLiPassion it works! Thanks.

@JiaLiPassion JiaLiPassion mentioned this pull request Apr 23, 2017
Closed
@sangecz
Copy link

sangecz commented Apr 23, 2017

is it fixed? which version? i tried downgrade from 0.8.8 to 0.8.5 and 0.8.4 but same issue as described in #6063

@dkontorovskyy
Copy link

@JiaLiPassion, do you know when /dist folder is going to be updated?

Thanks!

@JiaLiPassion
Copy link
Collaborator Author

@dkontorovskyy , it will be the next version release, please wait for a while.

@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Apr 24, 2017
Closed
This was referenced Apr 24, 2017
Closed
Closed
Closed
@intellix intellix mentioned this pull request Apr 25, 2017
Closed
@mhevery mhevery merged commit e598310 into angular:master Apr 25, 2017
bkbooth added a commit to loveisourweapon/liow2-client that referenced this pull request Apr 26, 2017
@bkbooth
Lock zone.js version to 0.8.5 temporarily
9551cd2 
* Fixes console error caused by zone.js@0.8.8
* angular/angular-cli#6036
* angular/zone.js#747
@JiaLiPassion JiaLiPassion deleted the issue-741 branch May 6, 2017 04:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mhevery mhevery mhevery approved these changes

@rweng rweng rweng approved these changes

@guodong guodong guodong approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot read property 'apply' of undefined window.onresize is not intercepted by zones
8 participants
@JiaLiPassion @smorhaim @sangecz @dkontorovskyy @mhevery @rweng @guodong @googlebot